Charts: honour prefers-reduced-motion across every Recharts series - #14
Draft
aurph wants to merge 1 commit into
Draft
Charts: honour prefers-reduced-motion across every Recharts series#14aurph wants to merge 1 commit into
aurph wants to merge 1 commit into
Conversation
Recharts animates every series by default and never consults the user's motion preference, so all 27 series on the site grew from zero regardless of what the visitor asked their OS for. The same default has a second effect that is easy to miss. Recharts builds series geometry inside react-smooth, which only emits the actual <path> on its first requestAnimationFrame tick. A chart that never receives a frame renders structurally complete but visually empty: the <g class="recharts-bar-rectangle"> groups are all there with nothing inside them. Nothing errors and nothing logs. That is what made these charts impossible to verify in a headless browser, and it is the same failure any visitor hits if rAF is throttled. - chart-theme.ts exports prefersReducedMotion and seriesAnimation, matching the existing axisProps/gridProps convention of a plain spreadable const. - Every Recharts <Bar>, <Line>, <Area>, <Pie> and <Radar> spreads it: 27 series across 9 files. - @visx series are deliberately excluded. PriceHistoryChart's <Area> comes from @visx/shape, which is a pure path generator with no animation and no such prop; passing it would be meaningless. The guard test walks client/src and fails if any Recharts series is added without the prop, because the failure mode is a silently blank chart rather than an error. Verified with Chrome --force-prefers-reduced-motion: the Compute Frontier operator bars now render real geometry, and their widths match the API to within rounding (132/88/83/71 px against 16245/11043/10460/8902 MW, ratios 1.00/0.67/0.63/0.54 vs 1.00/0.68/0.64/0.55). 303 tests pass, tsc clean, client build passes.
Owner
Author
|
Verified against current
Confirmed the second effect you describe: on the merged branch the compute-frontier bar and timeline charts render with visible geometry in a headless Chrome screenshot. The source-walking test is the valuable part here, since it fails on any future series added without the prop rather than letting the chart go silently blank. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Two bugs, one cause
Recharts animates every series by default and never consults the user's motion preference. All 27 series on the site grew from zero regardless of what the visitor asked their OS for. That is the accessibility bug.
The second effect is subtler and is why the compute-page charts could never be verified. Recharts builds series geometry inside
react-smooth, which only emits the actual<path>on its firstrequestAnimationFrametick. A chart that never receives a frame renders structurally complete but visually empty — the<g class="recharts-bar-rectangle">groups are all present with nothing inside:Nothing errors. Nothing logs. The chart just looks blank. Any visitor whose rAF is throttled hits the same thing.
Change
chart-theme.tsexportsprefersReducedMotionandseriesAnimation, following the existingaxisProps/gridPropsconvention of a plain spreadable const.<Bar>,<Line>,<Area>,<Pie>,<Radar>spreads it — 27 series across 9 files.@visxdeliberately excluded.PriceHistoryChart's<Area>comes from@visx/shape, a pure path generator with no animation and no such prop. Passing it would be meaningless, so the guard test skips files that don't import fromrecharts.The guard test
The failure mode is a silently blank chart, not an error, so a new chart added without the prop would regress this invisibly.
chart-motion.test.tswalksclient/srcand fails listing any offender. It found 9 series I missed on the first pass (PortfolioOverlay,TiltOverview) plus correctly flagged the visx false positive, which is what scoped the rule.Verification
Run with
chrome --force-prefers-reduced-motion, the Compute Frontier operator bars render real geometry, and the widths match the API to within rounding:This closes the "charts unverifiable in headless" gap I flagged on #12/#13 — they were correct all along, just unprovable.
npm test303 pass, 0 failnpx tsc --noEmitcleannpm run buildpassesIndependent of #12 and #13; touches no files either of them touch.